HYPERFLEET-1147 - feat: add caller identity support for audit attribution#120
HYPERFLEET-1147 - feat: add caller identity support for audit attribution#120kuudori wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (14)
✅ Files skipped from review due to trivial changes (3)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR introduces caller identity configuration and audit attribution to the hyperfleet E2E test framework. Configuration schema defines identity header, value, and optional JWT token. The client wiring layer extends the OpenAPI client to accept a request editor that injects identity credentials into each outgoing HTTP request. Test helpers expose the configured identity and provide a Gomega matcher to validate audit fields on Cluster and Resource objects. E2E lifecycle tests for cluster creation and deletion now assert that audit fields match the configured caller identity, with assertions skipped when identity is not configured. Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant Helper
participant OpenAPIClient
participant API
TestRunner->>Helper: request client (uses config)
Helper->>OpenAPIClient: create client with RequestEditor
OpenAPIClient->>API: send HTTP request (adds X-HyperFleet-Identity / Authorization)
API->>OpenAPIClient: respond (resource with audit CreatedBy/DeletedBy)
OpenAPIClient->>TestRunner: return resource
TestRunner->>Helper: assert HaveAuditIdentity(expected)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/hyperfleet-e2e/main.go`:
- Around line 33-34: Add a new CLI flag binding for the identity token so
token-only usage is supported: declare or reuse the variable identityToken and
register it with pfs.StringVar(&identityToken, "identity-token", "", "Caller
identity token (alternative to identity-header/identity-value)"), and add the
same binding in the other flagset where identityHeader/identityValue are
registered (the other pfs.StringVar calls referenced in the comment). Update any
flag parsing logic that constructs request identity to prefer identityToken when
set (instead of header/value).
In `@cmd/hyperfleet-e2e/test/cmd.go`:
- Around line 60-67: The viper.BindPFlag calls for binding parent flags are
ignoring returned errors; update the block that calls viper.BindPFlag (for
config.API.URL, config.Log.Level, config.Log.Format, config.Log.Output,
config.Identity.Header, config.Identity.Value using parentFlags.Lookup("..."))
to check each error and handle it (e.g., return the error from the surrounding
function or log/fatal with context) instead of discarding; ensure you capture
the returned error from each viper.BindPFlag call and include a clear message
referencing the flag/key when reporting the failure.
In `@e2e/cluster/delete.go`:
- Around line 56-58: When h.ExpectedIdentity() returns a non-empty expected
identity, don't just assert deletedCluster.DeletedBy is non-nil — assert it
equals the expected identity for end-to-end attribution. Update the test in
delete.go to (1) fetch expected := h.ExpectedIdentity(), (2) ensure
deletedCluster.DeletedBy is present, and then (3) compare the DeletedBy value
against expected (use the appropriate field/representation on
deletedCluster.DeletedBy) so the assertion fails if the API attributes deletion
to the wrong caller.
In `@pkg/config/config.go`:
- Around line 497-499: The log currently emits c.Identity.Value as
"identity_value" in plaintext using valueOrNotSet; change this to a redacted
form (reuse or add a helper like redactValue similar to redactToken) so
identity_value does not reveal emails/identifiers in logs; update the call in
the block that builds the map (replace valueOrNotSet(c.Identity.Value) with a
redaction helper) and ensure redactToken(c.Identity.Token) remains unchanged.
- Around line 462-465: In Validate(), after the existing header/value presence
check (the block referencing c.Identity.Header and c.Identity.Value), add a
format validation for c.Identity.Value so malformed audit identities fail fast;
call or implement a small validator (e.g., parseAuditIdentity(...) or a tailored
regexp) and return a descriptive error from Validate() if it doesn't match the
expected audit-identity format (include the invalid value in the fmt.Errorf
message) so config loading fails early rather than at API runtime.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: f6bf6e8a-4652-469d-9a0d-4495a6cd0f70
📒 Files selected for processing (14)
AGENTS.mdcmd/hyperfleet-e2e/main.gocmd/hyperfleet-e2e/test/cmd.goconfigs/config.yamldeploy-scripts/lib/api.shdocs/getting-started.mddocs/local-kind-setup.mde2e/cluster/creation.goe2e/cluster/delete.gopkg/client/client.gopkg/config/config.gopkg/helper/helper.gopkg/helper/matchers.gopkg/helper/suite.go
…tion Inject caller identity headers into all E2E API requests so tests work against an API with identity enforcement enabled (production default). - Add IdentityConfig (header/value/token) to config with CLI flags, env vars (HYPERFLEET_IDENTITY_*), and config.yaml support - Inject identity via OpenAPI RequestEditorFn in helper.newHelper() - Add HaveAuditIdentity matcher and ExpectedIdentity() helper - Add created_by assertion in cluster creation, deleted_by in deletion - Deploy API with identity_header enabled in local kind setup - Update AGENTS.md, getting-started, and local-kind-setup docs
e286b25 to
5d8c6b8
Compare
|
@kuudori: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Inject caller identity headers into all E2E API requests so tests work against an API with identity enforcement enabled (production default).
Summary
Test Plan
make test-allpassesmake lintpassesmake test-helm(if applicable)